-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update tests for Qiskit 1.0 #41
Conversation
This commit updates the execute tests to remove the use of execute() which was removed in Qiskit 1.0 after being deprecated in 0.46. How this managed to merge, I'm not entirely clear on because it should have failed CI on the removal PR, so there is likely a configuration issue in the neko custom action as well that will need to be fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The execute
changes all look totally fine, but I'm not sure if bringing in Runtime to Neko is going to be safe long term. Do you think it could be worth dropping the backend_selection
ability of the Aer plugin to get specific IBM-like devices? On a quick scan, I couldn't see any repos actually using the functionality.
@@ -4,6 +4,7 @@ qiskit-aer | |||
qiskit-nature[pyscf] | |||
qiskit-experiments | |||
qiskit-machine-learning | |||
qiskit-ibm-runtime>=0.19 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a potential problem here that Runtime doesn't use Neko, so could merge breaking changes to the Neko actions unknowingly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During my searching for backend_selection
, I also realised that qiskit-algorithms
doesn't have a Neko job in CI, so could also break it without us knowing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it could be worth dropping the backend_selection ability of the Aer plugin to get specific IBM-like devices? On a quick scan, I couldn't see any repos actually using the functionality.
Is there a potential problem here that Runtime doesn't use Neko, so could merge breaking changes to the Neko actions unknowingly?
Yeah, we should add a CI job to qiskit-ibm-runtime if we go this path. We could potentially remove this functionality and just rely on users to make their own backend plugin to use the fake backends. I was using this locally to test things out when I was first getting neko setup, but I don't know how widely used this part of the interface was. The plugin interface supports it but each plugin doesn't need to implement name selection.
During my searching for backend_selection, I also realised that qiskit-algorithms doesn't have a Neko job in CI, so could also break it without us knowing.
Yeah we should add a job to qiskit-algorithms, we can ask @woodsp-ibm or @ElePT about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well we do have a jobs in Algorithms that run all the unit tests against Qiskit Main so at least from that side we can see and it did turn up an issue along the way to the final 1.0. They are not required for CI, all that is is done against stable release, but they are run in each PR and in the nightly CI run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neko's a bit different to testing against Qiskit main
, though - Algorithms running Neko in CI is / would be just as much a protection for all the other Neko users as for Algorithms itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've proposed a PR to qiskit-ibm-runtime here: Qiskit/qiskit-ibm-runtime#1422 which adds a matching job. We need to update the custom action to leverage the backend selection mechanism to use the fake backends in the package. But that will have to come after this PR as this is necessary to unblock CI for everyone.
In Qiskit/qiskit-neko#41 we're updating the default simulator backend plugin used by qiskit-neko to rely on the fake backends provider in the qiskit-ibm-runtime project. A key aspect of qiskit-neko is the reciprocity of testing to maintain compatibility in the qiskit ecosystem (see: https://github.com/Qiskit/qiskit-neko?tab=readme-ov-file#downstream-usage-in-testing) so to ensure this is maintainable and the interface remains compatible for every project using neko this commit adds a corresponding job to exercise the qiskit-ibm-runtime usage in the tests. However, this is blocked until after Qiskit/qiskit-neko#41 merges as that is needed to fix the test suite. A followup will be needed to add a backend selection to the action configuration so that we can actually use a fake backend. A longer term step we should investigate is adding a qiskit-neko plugin to the repo which would enable using qiskit-ibm-runtime's backends and primitives directly for integration testing with real backends. But that is much longer term and isn't part of this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, this is fine by me - getting everyone's CI less angry immediately is a much better state of affairs, and we can worry about stabilising it against potential breakage afterwards.
In Qiskit/qiskit-neko#41 we're updating the default simulator backend plugin used by qiskit-neko to rely on the fake backends provider in the qiskit-ibm-runtime project. A key aspect of qiskit-neko is the reciprocity of testing to maintain compatibility in the qiskit ecosystem (see: https://github.com/Qiskit/qiskit-neko?tab=readme-ov-file#downstream-usage-in-testing) so to ensure this is maintainable and the interface remains compatible for every project using neko this commit adds a corresponding job to exercise the qiskit-ibm-runtime usage in the tests. However, this is blocked until after Qiskit/qiskit-neko#41 merges as that is needed to fix the test suite. A followup will be needed to add a backend selection to the action configuration so that we can actually use a fake backend. A longer term step we should investigate is adding a qiskit-neko plugin to the repo which would enable using qiskit-ibm-runtime's backends and primitives directly for integration testing with real backends. But that is much longer term and isn't part of this Co-authored-by: Elena Peña Tapia <[email protected]>
This commit updates the execute tests to remove the use of execute() which was removed in Qiskit 1.0 after being deprecated in 0.46. How this managed to merge, I'm not entirely clear on because it should have failed CI on the removal PR, so there is likely a configuration issue in the neko custom action as well that will need to be fixed.